Skip to content

fix(toolkit): lead every adapter message with its engine's own __engine_name__ - #2399

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2304-toolkit-v2
Oct 27, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2304-toolkit-v2

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2304.

  • Problem: the pcapkit.toolkit adapters put the engine name mid-sentence instead of leading with it, unlike the <prefix>: <lowercase description>: … {value!r} form fix(ethernet,pcapng): reject a non-string MAC or EUI address (#2222) #2232/fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) #2257 established, and hand-typed the name rather than reading it off the engine class's own __engine_name__.
  • Fix: each adapter module gains _ENGINE = <EngineClass>.__engine_name__ (plus the import), so a rename in pcapkit/foundation/engines/ can't leave the adapter spelling the old name. 4 modules, 5 sub-bugs:
    • pcap_ct.py/pypcap.py: "IPv4 reassembly is not supported by the pcap-ct engine: …" → "PCAP_CT: IPv4 reassembly is not supported: …" (and the PyPCAP sibling).
    • pypcapfile.py: "invalid PyPCAPFile IPv4 address: 5" → "PyPCAPFile: invalid IPv4 address: 5" (same for MAC address); the IPv6 UnsupportedCall and the "Frame N: TCP over IPv6 …" warning get the same treatment.
    • pyshark.py: boolean-field, frame.time_epoch and no-frame-layer messages reordered the same way.
    • scapy.py: the import-time warning reordered; the quoted package name is lowercased to 'scapy' to match the actual failing import and the sibling dependency warnings elsewhere in the codebase, which quote the installable name (flagging this as a judgment call — nothing pinned the old 'Scapy' spelling, so it was free to change).
  • No import cycle: the engine classes' toolkit imports are all function-local, so the new toolkit → foundation.engines import introduces none; verified both import orders and a plain import pcapkit.
  • Test: tests/toolkit/test_engine_message_prefix_2304_unit.py (9 tests) pins each message whole and asserts each module's _ENGINE is the engine class's own attribute. Existing pins in the four engines' test files updated.
  • Fail-without-fix: reverted the 5 adapter files only; new test module fails all 9, existing updated pins add up to 48 failed / 75 passed across the 5 affected test files. 0 failed with the fix.

…ne_name__ (#2304)

The pcapkit.toolkit adapters put the engine name mid-sentence instead of
leading with it, unlike the <prefix>: <lowercase description>: ...
{value!r} form #2232/#2257 established, and hand-typed the name rather
than reading it off the engine class's own __engine_name__.

Each adapter module gains _ENGINE = <EngineClass>.__engine_name__ (plus
the import), so a rename in pcapkit/foundation/engines/ cannot leave the
adapter spelling the old name. Four modules, five sub-bugs:

- pcap_ct.py/pypcap.py: "IPv4 reassembly is not supported by the pcap-ct
  engine: ..." -> "PCAP_CT: IPv4 reassembly is not supported: ..." (and
  the PyPCAP sibling).
- pypcapfile.py: "invalid PyPCAPFile IPv4 address: 5" ->
  "PyPCAPFile: invalid IPv4 address: 5" (same for MAC address); the IPv6
  UnsupportedCall and the "Frame N: TCP over IPv6 ..." warning get the
  same treatment.
- pyshark.py: boolean-field, frame.time_epoch and no-frame-layer messages
  all reordered the same way.
- scapy.py: the import-time warning reordered; the quoted package name
  is lowercased to 'scapy' to match the actual failing import and the
  sibling dependency warnings elsewhere in the codebase, which quote the
  installable name.

The engine classes' toolkit imports are all function-local, so the new
toolkit -> foundation.engines import introduces no cycle.

Tests: new tests/toolkit/test_engine_message_prefix_2304_unit.py (9
tests) pins each message whole and asserts each module's _ENGINE is the
engine class's own attribute. Existing pins in the four engines' test
files updated for the new text.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 27, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at e73ba4be1 (Sonnet). Confirmed _ENGINE is genuinely the same object as each engine class's own __engine_name__ in all 5 modules (PCAP_CT, PyPCAP, PyPCAPFile, PyShark, Scapy). No runtime import cycle (14 fresh-interpreter import orders clean; nothing module-level reaches toolkit from the engines package). Message pairs verified on both trees. 216 passed/813 subtests; mypy/pylint identical modulo line shift. All 9 new tests fail against unfixed adapters.

Two small non-blocking notes: the PR body's "48 failed / 75 passed" reads as 50/73 under independent measurement (same 123 total — minor arithmetic slip, not a correctness issue), and it says "4 modules" where 5 adapter files actually changed. Also advisory-only: pylint's R0401 cyclic-import check (counts function-local imports, which CI treats as advisory) rises from 0 to 5 when engines+toolkit are linted jointly — runtime has no cycle, this is a lint-tool artifact the repo already tolerates elsewhere.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 27, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 27, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.68% (unit tier, Python 3.15, e73ba4be1, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19601 880 2620 867 92.12%
pcapkit/corekit 2596 74 818 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3044 23 1060 25 98.83%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17482 209 4602 178 98.24%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4434 1961 1018 167 52.49%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 3807439 into main Oct 27, 2026
68 checks passed
@JarryShaw
JarryShaw deleted the fix/2304-toolkit-v2 branch October 27, 2026 22:32
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 27, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 28, 2026
JarryShaw added a commit that referenced this pull request Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Two pairs share a
bullet, since each is the direct follow-up of the other: #2393 with
#2401, and #2405 with #2407. The diff on the entry file is purely
additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on the seven classes owning
a __proto__ registry (#2392), __proto_dest__ on the four enum registries
and Vendor.PROTO_DEST to generate it (#2403). Both PRs added the autodoc
entries, so both are rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
JarryShaw added a commit that referenced this pull request Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Three pairs share a
bullet, since each is the direct follow-up or counterpart of the other:
#2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the
entry file is purely additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on each class owning a
__proto__ registry (#2392), __proto_dest__ on the enum registries and
Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes
and registries rather than counting them, per the no-counted-numbers rule
for the unreleased entry. Both PRs added the autodoc entries, so both are
rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
JarryShaw added a commit that referenced this pull request Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only
#2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and
#2398 and #2403 only reworded lines that were already there. Every other
user-visible change in the range -- wire-semantics fixes, maker guards,
reassembly behaviour, public-API and error-message changes -- went
unrecorded, so the entry no longer describes the release.

Fix: one entry per user-visible pull request, in each module section's
existing style and ordered by the issue it closes. Three pairs share a
bullet, since each is the direct follow-up or counterpart of the other:
#2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the
entry file is purely additive.

Internal-only work is left out deliberately: dependency and action bumps
(#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379,
#2380, #2402), the unreachable-branch removal (#2378), and the #: comment
marker the HIP Group and Packet crawlers now write (#2381). That last one
renders nothing either way: the marker lands inside the generated
_missing_ body, conf.py's exclude-members drops _missing_ from autodoc,
and a comment in a method body is not an attribute comment to begin with.

#2392 and #2403 are recorded together, under Added rather than Fixed, for
the declarations they introduce: __proto_key__ on each class owning a
__proto__ registry (#2392), __proto_dest__ on the enum registries and
Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes
and registries rather than counting them, per the no-counted-numbers rule
for the unreleased entry. Both PRs added the autodoc entries, so both are
rendered public attributes. #2392's rejection-message
change is deliberately not recorded as a fix: the bare "code must be ..."
text it prefixed came from _check_code, which #2193 introduced and which
is in no release tag -- absent at v1.5.0b14 -- so no released version ever
emitted it. #2403 is no longer skipped as internal because it reworded an
existing bullet to name __proto_dest__ while nothing credited the
attribute's introduction.

Four claims are corrected against the source rather than carried over:
FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not
1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback
read cmmd.value, so a .value-carrying object was accepted unchecked
rather than raising; #2399 left the dpkt adapter and the adapters'
ModuleNotFound raises alone, so it claims no universal prefix; and
#2397's old tail did name the version it got, so the entry quotes it and
gains the :issue:`2302` it was missing. #2370's "a Section Header Block
... still fails" sentence is dropped: #2369 settles the byte order first,
and an SHB built from raw octets with ctx=None now rebuilds byte-exact in
both byte orders.

CHANGELOG.md is regenerated with util/changelog_md.py.

Test: tests/project/ in full, 0 failures, including test_changelog_md.py's
in-step and supported-subset checks; util/changelog_md.py --check reports
the two files in step; the entry parses under docutils with no structural
warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(toolkit): adapter messages put the engine name mid-sentence, unlike __engine_name__

1 participant